Add stub for new libglnx tmpfile API, port simpler callers to it

It's hard right now to do a full port to the new libglnx tmpfile
API since there are complex cases in the commit path which deal
with symlinks as well.

Let's make things more gradual by introducing the important part (struct with
autocleanup) here in libotutil, port what we can. This will make a future
complete port easier.

Closes: #871
Approved by: jlebon

diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c
index f6893fd098da119dd7638fa5910c69053a99b9f4..d1dab0959563d244de34a4f1e99ac28cabc7ae48 100644 (file)
--- a/src/libostree/ostree-fetcher-curl.c
+++ b/src/libostree/ostree-fetcher-curl.c
@@ -99,8 +99,7 @@ struct FetcherRequest {
   OstreeFetcherRequestFlags flags;
   gboolean is_membuf;
   GError *caught_write_error;
-  char *out_tmpfile;
-  int out_tmpfile_fd;
+  OtTmpfile tmpf;
   GString *output_buf;
 
   CURL *easy;
@@ -269,12 +268,11 @@ request_get_uri (FetcherRequest *req, guint idx)
 static gboolean
 ensure_tmpfile (FetcherRequest *req, GError **error)
 {
-  if (req->out_tmpfile_fd == -1)
+  if (!req->tmpf.initialized)
     {
-      if (!glnx_open_tmpfile_linkable_at (req->fetcher->tmpdir_dfd, ".",
-                                          O_WRONLY | O_CLOEXEC, &req->out_tmpfile_fd,
-                                          &req->out_tmpfile,
-                                          error))
+      if (!ot_open_tmpfile_linkable_at (req->fetcher->tmpdir_dfd, ".",
+                                        O_WRONLY | O_CLOEXEC, &req->tmpf,
+                                        error))
         return FALSE;
     }
   return TRUE;
@@ -387,18 +385,14 @@ check_multi_info (OstreeFetcher *fetcher)
                 {
                   g_task_return_error (task, g_steal_pointer (&local_error));
                 }
-              else if (fchmod (req->out_tmpfile_fd, 0644) < 0)
+              else if (fchmod (req->tmpf.fd, 0644) < 0)
                 {
                   glnx_set_error_from_errno (error);
                   g_task_return_error (task, g_steal_pointer (&local_error));
                 }
-              else if (!glnx_link_tmpfile_at (fetcher->tmpdir_dfd,
-                                         GLNX_LINK_TMPFILE_REPLACE,
-                                         req->out_tmpfile_fd,
-                                         req->out_tmpfile,
-                                         fetcher->tmpdir_dfd,
-                                         tmpfile_path,
-                                         error))
+              else if (!ot_link_tmpfile_at (&req->tmpf, GLNX_LINK_TMPFILE_REPLACE,
+                                            fetcher->tmpdir_dfd, tmpfile_path,
+                                            error))
                 g_task_return_error (task, g_steal_pointer (&local_error));
               else
                 {
@@ -586,8 +580,8 @@ write_cb (void *ptr, size_t size, size_t nmemb, void *data)
     {
       if (!ensure_tmpfile (req, &req->caught_write_error))
         return -1;
-     g_assert (req->out_tmpfile_fd >= 0);
-      if (glnx_loop_write (req->out_tmpfile_fd, ptr, realsize) < 0)
+      g_assert (req->tmpf.fd >= 0);
+      if (glnx_loop_write (req->tmpf.fd, ptr, realsize) < 0)
         {
           glnx_set_error_from_errno (&req->caught_write_error);
           return -1;
@@ -622,9 +616,7 @@ request_unref (FetcherRequest *req)
   g_ptr_array_unref (req->mirrorlist);
   g_free (req->filename);
   g_clear_error (&req->caught_write_error);
-  if (req->out_tmpfile_fd != -1)
-    (void) close (req->out_tmpfile_fd);
-  g_free (req->out_tmpfile);
+  ot_tmpfile_clear (&req->tmpf);
   if (req->output_buf)
     g_string_free (req->output_buf, TRUE);
   curl_easy_cleanup (req->easy);
@@ -847,7 +839,6 @@ _ostree_fetcher_request_async (OstreeFetcher         *self,
   /* We'll allocate the tmpfile on demand, so we handle
    * file I/O errors just in the write func.
    */
-  req->out_tmpfile_fd = -1;
   if (req->is_membuf)
     req->output_buf = g_string_new ("");
 

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
index f6e91498f3565bb3ae30b1191edc599d0cb7fd9b..360c939f8541c72efed64e41bea8794411b93453 100644 (file)
--- a/src/libostree/ostree-repo-checkout.c
+++ b/src/libostree/ostree-repo-checkout.c
@@ -60,13 +60,11 @@ checkout_object_for_uncompressed_cache (OstreeRepo      *self,
   guint32 file_mode = g_file_info_get_attribute_uint32 (src_info, "unix::mode");
   file_mode &= ~(S_ISUID|S_ISGID);
 
-  glnx_fd_close int fd = -1;
-  g_autofree char *temp_filename = NULL;
-  if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY | O_CLOEXEC,
-                                      &fd, &temp_filename,
-                                      error))
+  g_auto(OtTmpfile) tmpf = { 0, };
+  if (!ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY | O_CLOEXEC,
+                                    &tmpf, error))
     return FALSE;
-  g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (fd, FALSE);
+  g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (tmpf.fd, FALSE);
 
   if (g_output_stream_splice (temp_out, content, 0, cancellable, error) < 0)
     return FALSE;
@@ -76,14 +74,14 @@ checkout_object_for_uncompressed_cache (OstreeRepo      *self,
 
   if (!self->disable_fsync)
     {
-      if (TEMP_FAILURE_RETRY (fsync (fd)) < 0)
+      if (TEMP_FAILURE_RETRY (fsync (tmpf.fd)) < 0)
         return glnx_throw_errno (error);
     }
 
   if (!g_output_stream_close (temp_out, cancellable, error))
     return FALSE;
 
-  if (fchmod (fd, file_mode) < 0)
+  if (fchmod (tmpf.fd, file_mode) < 0)
     return glnx_throw_errno (error);
 
   if (!_ostree_repo_ensure_loose_objdir_at (self->uncompressed_objects_dir_fd,
@@ -91,10 +89,9 @@ checkout_object_for_uncompressed_cache (OstreeRepo      *self,
                                             cancellable, error))
     return FALSE;
 
-  if (!glnx_link_tmpfile_at (self->tmp_dir_fd, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST,
-                             fd, temp_filename,
-                             self->uncompressed_objects_dir_fd, loose_path,
-                             error))
+  if (!ot_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST,
+                           self->uncompressed_objects_dir_fd, loose_path,
+                           error))
     return FALSE;
 
   return TRUE;
@@ -185,7 +182,6 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
                                 GCancellable   *cancellable,
                                 GError        **error)
 {
-  g_autofree char *temp_filename = NULL;
   const gboolean union_mode = options->overwrite_mode == OSTREE_REPO_CHECKOUT_OVERWRITE_UNION_FILES;
   const gboolean add_mode = options->overwrite_mode == OSTREE_REPO_CHECKOUT_OVERWRITE_ADD_FILES;
   const gboolean sepolicy_enabled = options->sepolicy && !repo->disable_xattrs;
@@ -252,7 +248,7 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
     }
   else if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
     {
-      glnx_fd_close int temp_fd = -1;
+      g_auto(OtTmpfile) tmpf = { 0, };
       guint32 file_mode;
       GLnxLinkTmpfileReplaceMode replace_mode;
 
@@ -261,9 +257,8 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
       if (options->mode == OSTREE_REPO_CHECKOUT_MODE_USER)
         file_mode &= ~(S_ISUID|S_ISGID);
 
-      if (!glnx_open_tmpfile_linkable_at (destination_dfd, ".", O_WRONLY | O_CLOEXEC,
-                                          &temp_fd, &temp_filename,
-                                          error))
+      if (!ot_open_tmpfile_linkable_at (destination_dfd, ".", O_WRONLY | O_CLOEXEC,
+                                        &tmpf, error))
         return FALSE;
 
       if (sepolicy_enabled)
@@ -274,11 +269,11 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
                                           g_file_info_get_attribute_uint32 (file_info, "unix::mode"),
                                           &label, cancellable, error))
             return FALSE;
-          if (fsetxattr (temp_fd, "security.selinux", label, strlen (label), 0) < 0)
+          if (fsetxattr (tmpf.fd, "security.selinux", label, strlen (label), 0) < 0)
             return glnx_throw_errno_prefix (error, "Setting security.selinux");
         }
 
-      if (!write_regular_file_content (repo, options, temp_fd, file_info, xattrs, input,
+      if (!write_regular_file_content (repo, options, tmpf.fd, file_info, xattrs, input,
                                        cancellable, error))
         return FALSE;
 
@@ -290,10 +285,9 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
       else
         replace_mode = GLNX_LINK_TMPFILE_NOREPLACE;
 
-      if (!glnx_link_tmpfile_at (destination_dfd, replace_mode,
-                                 temp_fd, temp_filename, destination_dfd,
-                                 destination_name,
-                                 error))
+      if (!ot_link_tmpfile_at (&tmpf, replace_mode,
+                               destination_dfd, destination_name,
+                               error))
         return FALSE;
     }
   else

diff --git a/src/libotutil/ot-fs-utils.c b/src/libotutil/ot-fs-utils.c
index 529077fb1e83e371c5d2590390ef884fbe4538ae..fe185e665464817506ea1a21ca4466fabb5baa69 100644 (file)
--- a/src/libotutil/ot-fs-utils.c
+++ b/src/libotutil/ot-fs-utils.c
@@ -25,6 +25,61 @@
 #include <sys/xattr.h>
 #include <gio/gunixinputstream.h>
 
+/* Before https://github.com/GNOME/libglnx/commit/9929adc, the libglnx
+ * tmpfile API made it hard to clean up tmpfiles in failure cases.
+ * it's API breaking. Carry the fix here until we're ready to fully port.
+ */
+void
+ot_tmpfile_clear (OtTmpfile *tmpf)
+{
+  if (!tmpf->initialized)
+    return;
+  if (tmpf->fd == -1)
+    return;
+  (void) close (tmpf->fd);
+  /* If ->path is set, we're likely aborting due to an error. Clean it up */
+  if (tmpf->path)
+    {
+      (void) unlinkat (tmpf->src_dfd, tmpf->path, 0);
+      g_free (tmpf->path);
+    }
+}
+
+gboolean
+ot_open_tmpfile_linkable_at (int dfd,
+                             const char *subpath,
+                             int flags,
+                             OtTmpfile *out_tmpf,
+                             GError **error)
+{
+  if (!glnx_open_tmpfile_linkable_at (dfd, subpath, flags, &out_tmpf->fd, &out_tmpf->path, error))
+    return FALSE;
+  out_tmpf->initialized = TRUE;
+  out_tmpf->src_dfd = dfd;
+  return TRUE;
+}
+
+gboolean
+ot_link_tmpfile_at (OtTmpfile *tmpf,
+                    GLnxLinkTmpfileReplaceMode mode,
+                    int target_dfd,
+                    const char *target,
+                    GError **error)
+{
+  g_return_val_if_fail (tmpf->initialized, FALSE);
+  glnx_fd_close int fd = glnx_steal_fd (&tmpf->fd);
+  if (!glnx_link_tmpfile_at (tmpf->src_dfd, mode, fd, tmpf->path,
+                             target_dfd, target, error))
+    {
+      if (tmpf->path)
+        (void) unlinkat (tmpf->src_dfd, tmpf->path, 0);
+      tmpf->initialized = FALSE;
+      return FALSE;
+    }
+  tmpf->initialized = FALSE;
+  return TRUE;
+}
+
 /* Convert a fd-relative path to a GFile* - use
  * for legacy code.
  */

diff --git a/src/libotutil/ot-fs-utils.h b/src/libotutil/ot-fs-utils.h
index 14df8acb16da46cc24d9fbe442ccf0c85059da1e..26c5a49978a02b3c84c2fe9efb4b717bcb99c192 100644 (file)
--- a/src/libotutil/ot-fs-utils.h
+++ b/src/libotutil/ot-fs-utils.h
@@ -25,6 +25,30 @@
 
 G_BEGIN_DECLS
 
+/* This is a copy of https://github.com/GNOME/libglnx/pull/46 until we
+ * can do a full port; see https://github.com/ostreedev/ostree/pull/861 */
+typedef struct {
+  gboolean initialized;
+  int src_dfd;
+  int fd;
+  char *path;
+} OtTmpfile;
+void ot_tmpfile_clear (OtTmpfile *tmpf);
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(OtTmpfile, ot_tmpfile_clear);
+
+gboolean
+ot_open_tmpfile_linkable_at (int dfd,
+                             const char *subpath,
+                             int flags,
+                             OtTmpfile *out_tmpf,
+                             GError **error);
+gboolean
+ot_link_tmpfile_at (OtTmpfile *tmpf,
+                    GLnxLinkTmpfileReplaceMode flags,
+                    int target_dfd,
+                    const char *target,
+                    GError **error);
+
 GFile * ot_fdrel_to_gfile (int dfd, const char *path);
 
 gboolean ot_readlinkat_gfile_info (int             dfd,

diff --git a/src/ostree/ot-remote-cookie-util.c b/src/ostree/ot-remote-cookie-util.c
index b80c825c106e65a76319857bffa2855f84691d9e..9e152e18fb586d3838c0ae36e678e3130a7134f4 100644 (file)
--- a/src/ostree/ot-remote-cookie-util.c
+++ b/src/ostree/ot-remote-cookie-util.c
@@ -202,8 +202,7 @@ ot_delete_cookie_at (int dfd, const char *jar_path,
 {
   gboolean found = FALSE;
 #ifdef HAVE_LIBCURL
-  glnx_fd_close int tempfile_fd = -1;
-  g_autofree char *tempfile_path = NULL;
+  g_auto(OtTmpfile) tmpf = { 0, };
   g_autofree char *dnbuf = NULL;
   const char *dn = NULL;
   g_autoptr(OtCookieParser) parser = NULL;
@@ -213,9 +212,8 @@ ot_delete_cookie_at (int dfd, const char *jar_path,
 
   dnbuf = g_strdup (jar_path);
   dn = dirname (dnbuf);
-  if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC,
-                                      &tempfile_fd, &tempfile_path,
-                                      error))
+  if (!ot_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC,
+                                    &tmpf, error))
     return FALSE;
 
   while (ot_parse_cookies_next (parser))
@@ -229,19 +227,14 @@ ot_delete_cookie_at (int dfd, const char *jar_path,
           continue;
         }
 
-      if (glnx_loop_write (tempfile_fd, parser->line, strlen (parser->line)) < 0 ||
-          glnx_loop_write (tempfile_fd, "\n", 1) < 0)
-        {
-          glnx_set_error_from_errno (error);
-          return FALSE;
-        }
+      if (glnx_loop_write (tmpf.fd, parser->line, strlen (parser->line)) < 0 ||
+          glnx_loop_write (tmpf.fd, "\n", 1) < 0)
+        return glnx_throw_errno_prefix (error, "write");
     }
 
-  if (!glnx_link_tmpfile_at (AT_FDCWD, GLNX_LINK_TMPFILE_REPLACE,
-                             tempfile_fd,
-                             tempfile_path,
-                             AT_FDCWD, jar_path,
-                             error))
+  if (!ot_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE,
+                           AT_FDCWD, jar_path,
+                           error))
     return FALSE;
 #else
   GSList *cookies;